Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update mda dependency #63

Merged
merged 6 commits into from
Nov 30, 2016
Merged

Update mda dependency #63

merged 6 commits into from
Nov 30, 2016

Conversation

kain88-de
Copy link
Contributor

This includes various fixes to be usable with the new MDAnalysis version. kwargs can't be stored right now because of an old bug that has reappeared in MDAnalysis.

@kain88-de
Copy link
Contributor Author

OK now everything runs with the new version of MDAnalysis.

@kain88-de
Copy link
Contributor Author

This should work now also with 0.15.0 and the new releases

@kain88-de kain88-de force-pushed the update-mda-dependency branch from 6f53c4f to d8b413c Compare November 8, 2016 22:50
@codecov-io
Copy link

Current coverage is 89.48% (diff: 100%)

Merging #63 into develop will decrease coverage by 0.14%

@@            develop        #63   diff @@
==========================================
  Files             7          7          
  Lines           357        352     -5   
  Methods           0          0          
  Messages          0          0          
  Branches         58         58          
==========================================
- Hits            320        315     -5   
  Misses           22         22          
  Partials         15         15          

Powered by Codecov. Last update 98f3db0...d8b413c

@kain88-de
Copy link
Contributor Author

The initial missing import problem is now fixed in MDAnalysis development branch. We should still include these fixes.

MDAnalysis/mdanalysis#1070

@kain88-de
Copy link
Contributor Author

@dotsdl ping

@@ -216,7 +215,11 @@ def _apply_resnums(self):
resnums = None

if resnums:
self._treant._universe.residues.set_resnums(np.array(resnums))
# Compatibility for MDAnalysis pre 0.16.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/MDAnalysis/mdanalysis/blob/release-0.14.0/package/MDAnalysis/core/AtomGroup.py#L1397

I think you can just switch to only the .resnums = resnums branch here, it was supported in 0.14

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we want to get rid of the storage of resnums here. The feature is hidden in MDS at the moment because we definitely won't be supporting it in the future, but it was really useful and important to work I was currently doing. With the new topology system in MDA and usage of persistent topologies we can completely eliminate this soon.

Thanks for this, @kain88-de. We keep it for now.

Copy link
Member

@dotsdl dotsdl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Looks great @kain88-de! Merging.

@@ -216,7 +215,11 @@ def _apply_resnums(self):
resnums = None

if resnums:
self._treant._universe.residues.set_resnums(np.array(resnums))
# Compatibility for MDAnalysis pre 0.16.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we want to get rid of the storage of resnums here. The feature is hidden in MDS at the moment because we definitely won't be supporting it in the future, but it was really useful and important to work I was currently doing. With the new topology system in MDA and usage of persistent topologies we can completely eliminate this soon.

Thanks for this, @kain88-de. We keep it for now.

@dotsdl dotsdl merged commit 452a420 into develop Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants